re-pushed, removing all fence related parts of this PR, since KVS fence support is now gone

re-pushed, removing all the fence stuff that is no longer relevant b/c KVS fence support was removed

Some small set of transaction stats

 "transaction-opcount": {
  "commit": {
   "count": 48829,
   "min": 1,
   "mean": 1.976673698007309,
   "stddev": 6.318251377545355,
   "max": 398
  }
 },

the max number of ops in a transaction is 398, which means my default cap of 64K seems more than fine. I'll still monitor stats until release time, but removing WIP for now so this can be reviewed

saw this today

 "transaction-opcount": {
  "commit": {
   "count": 95589,
   "min": 1,
   "mean": 3.4195984893659341,
   "stddev": 42.249674028063289,
   "max": 11395
  }
 },

max of 11K ... perhaps upping to 128K over 64K would still be good but a bit safer.

Before we do this, we should probably investigate why we are seeing these high op-count commits.

I think stdout events are batched by this code, which triggers the commit of a batch of events based on a timer. Maybe it should also have a high water mark on the operation count and/or the cumulative size of the events.

https://github.com/flux-framework/flux-core/blob/master/src/common/libeventlog/eventlogger.c

If we can make sure flux-core code is well behaved, then it makes sense to me to impose a limit to avoid regressions and bad behavior by framework projects, but I should think we could set it much lower than 128K. We should also provide some way for API users to know what the limit is. For example, then the above code could set its high water mark accordingly. Maye we could even just make it a constant in kvs.h.

Note this PR in the title and a few other places uses "transaction" where "operation" is meant. IMHO the title should be "kvs: limit the number of operations per commit".

Codecov Report

Attention: Patch coverage is 90.90909% with 2 lines in your changes missing coverage. Please review.

Project coverage is 83.84%. Comparing base (3323cf0) to head (a93874b).
Report is 1 commits behind head on master.

@@           Coverage Diff           @@
##           master    #6581   +/-   ##
=======================================
  Coverage   83.83%   83.84%           
=======================================
  Files         539      539           
  Lines       90283    90305   +22     
=======================================
+ Hits        75693    75713   +20     
- Misses      14590    14592    +2     

If we can make sure flux-core code is well behaved, then it makes sense to me to impose a limit to avoid regressions and bad behavior by framework projects, but I should think we could set it much lower than 128K. We should also provide some way for API users to know what the limit is. For example, then the above code could set its high water mark accordingly. Maye we could even just make it a constant in kvs.h.

Good point. I'll write up an issue to investigate this issue as well. However, this PR was initially developed under the idea to defend against a denial of service, so I think it is worthwhile to have the max independent of it. At this moment, a rogue user (or misbehaving code) could create a KVS transaction with like a bajillion operations in it right now.